Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create time syncronization protocol #7007

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

mcm001
Copy link
Contributor

@mcm001 mcm001 commented Aug 26, 2024

A very rough first pass at splitting the time sync requirements out into their own UDP thing. Putting this up early with a bunch of todos for visibility and to solicit feedback

[[format]]
=== Message Format

TODO: Should I have an arbitrary magic value? do i need checksums? should we include a sequence number or other way to check if a message was replied to besides checking if we got the timestamp we expected back?
Copy link
Member

@rzblue rzblue Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client can treat the client timestamp as a sequence number if it's monotonic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented using this, and having the client throw out all pongs that don't match the currently active ping. This does mean that if you send pings faster than the server can handle, you might just never get your reply back since it'll get swapped out, which isn't great.

@rzblue
Copy link
Member

rzblue commented Aug 26, 2024

We need to be careful about using kernel timestamping- it uses the system clock, which can jump forward and backwards if the time is adjusted (e.g. on initial DS connection)

@mcm001
Copy link
Contributor Author

mcm001 commented Aug 26, 2024

Yeah, it might be best to drop that feature for now.

@mcm001 mcm001 changed the title Create time syncronization protocol documentation Create time syncronization protocol Sep 1, 2024
@mcm001 mcm001 marked this pull request as ready for review September 2, 2024 00:09
@mcm001 mcm001 requested a review from a team as a code owner September 2, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants